Closed
Bug 1116910
Opened 10 years ago
Closed 9 years ago
Share button is larger than other buttons in ActionBar action mode on new tablet
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, defect)
Tracking
(firefox36 verified, firefox37 verified, firefox38 verified)
VERIFIED
FIXED
Firefox 38
People
(Reporter: mcomella, Assigned: mcomella)
References
Details
Attachments
(8 files, 4 obsolete files)
7.31 KB,
application/zip
|
Details | |
66.88 KB,
image/png
|
antlam
:
feedback+
|
Details |
5.46 KB,
patch
|
capella
:
review+
Sylvestre
:
approval-mozilla-aurora+
Sylvestre
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
76.63 KB,
image/png
|
Details | |
70.97 KB,
image/png
|
Details | |
236.59 KB,
image/png
|
Details | |
165.09 KB,
image/png
|
Details | |
269.62 KB,
image/png
|
Details |
Found from bug 1111598. See [1] for screenshot. Low priority as it doesn't look too terrible. [1]: https://bug1111598.bugzilla.mozilla.org/attachment.cgi?id=8536567
Duplicate of Bug 1026436?
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
Seems the issue here is mis-matched icon sizes. On xhdpi, expected: drawable-xhdpi/ab_copy.png PNG 64x64 64x64+0+0 8-bit sRGB 185B 0.000u 0:00.000 Actual: drawable-xhdpi/ic_menu_share.png PNG 84x84 84x84+0+0 8-bit sRGB 711B 0.000u 0:00.000 Anthony, can I get some assets for the share button? [1] Notably, this issue does not affect phone, probably because the toolbar buttons centerInside [2] but the tablet toolbar is a larger height than on phones so the icon appears to grow. Alternatively, I can add some padding, which would prevent the icon from scaling too much. [1]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/drawable-xhdpi/ic_menu_share.png [2]: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/resources/values-v11/styles.xml?rev=7be62fa11c20#96
Flags: needinfo?(alam)
Assignee | ||
Comment 4•10 years ago
|
||
I think we got trolled! The action bar assets contain whitespace (e.g. [1]). I filed bug 1122752 to remove this whitespace but it'd be faster and easier just to match the whitespace in the existing assets for this share asset. Anthony, do you mind providing some new assets?
Flags: needinfo?(alam)
Comment 5•10 years ago
|
||
Adding padding!
Attachment #8550492 -
Attachment is obsolete: true
Flags: needinfo?(alam)
Assignee | ||
Comment 6•10 years ago
|
||
Assignee | ||
Updated•10 years ago
|
Attachment #8550602 -
Attachment is obsolete: true
Assignee | ||
Comment 7•10 years ago
|
||
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8550602 [details] Hamburger menu Note that the hamburger menu's share is thus a different style than the action bar - are you okay with this, Anthony? If so, I filed bug 1122826 to consolidate these styles (because it could require some work).
Attachment #8550602 -
Attachment description: (Pre-patch) menu → Hamburger menu
Attachment #8550602 -
Attachment is obsolete: false
Attachment #8550602 -
Flags: feedback?(alam)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8550617 -
Flags: review?(markcapella)
Assignee | ||
Comment 10•10 years ago
|
||
We can't actually use a different icon for the action bar on phone and tablet so let's just put it everywhere (i.e. bug 1122826). f? for this and the next five images - you can see the existing icons in your local builds.
Attachment #8550602 -
Attachment is obsolete: true
Attachment #8550616 -
Attachment is obsolete: true
Attachment #8550602 -
Flags: feedback?(alam)
Attachment #8550632 -
Flags: feedback?(alam)
Assignee | ||
Comment 11•10 years ago
|
||
capella pointed out via IRC that the old patch breaks on phones - let's just replace the old icon then.
Attachment #8550634 -
Flags: review?(markcapella)
Assignee | ||
Updated•10 years ago
|
Attachment #8550617 -
Attachment is obsolete: true
Attachment #8550617 -
Flags: review?(markcapella)
Assignee | ||
Comment 12•10 years ago
|
||
Assignee | ||
Comment 13•10 years ago
|
||
Assignee | ||
Comment 14•10 years ago
|
||
Assignee | ||
Comment 15•10 years ago
|
||
Assignee | ||
Comment 16•10 years ago
|
||
Assignee | ||
Comment 17•10 years ago
|
||
I can confirm that my 9" tablet (N9) looks similar to the 7" version.
Comment 18•10 years ago
|
||
Comment on attachment 8550634 [details] [diff] [review] Add new share icons in the action bar for tablet Review of attachment 8550634 [details] [diff] [review]: ----------------------------------------------------------------- wfm - the difference is noticeable on my tablet, but on my phone I don't see it :-/
Attachment #8550634 -
Flags: review?(markcapella) → review+
Comment 19•10 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #4) > I think we got trolled! The action bar assets contain whitespace (e.g. [1]). > I filed bug 1122752 to remove this whitespace but it'd be faster and easier > just to match the whitespace in the existing assets for this share asset. Thank you for filing bug 1122752. Ideally, we don't want padding.
Comment 20•9 years ago
|
||
Comment on attachment 8550632 [details]
(Post-patch) Phone action bar
+ nice work!
But I think it's slightly larger than the other icons in the AB.. does that matter with all the other work we're doing around removing the white space?
Attachment #8550632 -
Flags: feedback?(alam) → feedback+
Updated•9 years ago
|
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 21•9 years ago
|
||
We decided in our weekly tablet meeting that the in-asset padding on the share button is probably not large enough on the top causing a misalignment, but it's not that noticeable so we're going to land as is. On holiday - please check in for me! :D Just an asset swap, no try run should be necessary.
Flags: needinfo?(michael.l.comella)
Keywords: checkin-needed
Assignee | ||
Comment 22•9 years ago
|
||
Thank you! https://hg.mozilla.org/integration/fx-team/rev/bc6777a08328
Keywords: checkin-needed
Assignee | ||
Comment 23•9 years ago
|
||
Comment on attachment 8550634 [details] [diff] [review] Add new share icons in the action bar for tablet Approval Request Comment [Feature/regressing bug #]: New tablet UI (bug 1014156) [User impact if declined]: Users will see a large share icon in the action bar on tablet, which looks unpolished [Describe test coverage new/current, TreeHerder]: None [Risks and why]: In the worst case, we swap the wrong assets and the share button is completely broken in all contexts (action bar, context menu, hamburger menu). However, it's otherwise low risk - we're just swapping some assets for some new ones. [String/UUID change made/needed]: N/A
Attachment #8550634 -
Flags: approval-mozilla-beta?
Attachment #8550634 -
Flags: approval-mozilla-aurora?
Comment 24•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bc6777a08328
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 38
Updated•9 years ago
|
Updated•9 years ago
|
Attachment #8550634 -
Flags: approval-mozilla-beta?
Attachment #8550634 -
Flags: approval-mozilla-beta+
Attachment #8550634 -
Flags: approval-mozilla-aurora?
Attachment #8550634 -
Flags: approval-mozilla-aurora+
Comment 26•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/1388e370eadc https://hg.mozilla.org/releases/mozilla-beta/rev/f6b2623900f1
Comment 27•9 years ago
|
||
Verified as fixed on latest Aurora and Nightly on Nexus 7 (Android 5.0.1) and Sony Xperia Z2 SGP511 (Android 4.4.4). The share button looks fine.
Comment 29•9 years ago
|
||
Verified as fixed in Firefox for Android 36 Beta 4; Device: Asus Transformer Pad TF300T (Android 4.2.1)
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•